feat: add NangoConnection<I> extractor for nango-powered integration routes#3962
feat: add NangoConnection<I> extractor for nango-powered integration routes#3962devin-ai-integration[bot] wants to merge 7 commits intomainfrom
Conversation
…routes - Add nango_connections supabase migration table - Add OwnedNangoProxy + OwnedNangoHttpClient to nango crate - Add NangoIntegrationId trait with GoogleCalendar/GoogleDrive marker types - Add NangoConnection<I> axum extractor (resolves connection_id from DB) - Extend webhook handler to upsert/delete nango_connections - Refactor api-calendar to use NangoConnection<GoogleCalendar> - Refactor api-storage to use NangoConnection<GoogleDrive> - Remove connection_id from request bodies (server-side lookup) - Remove duplicated config.rs/state.rs from api-calendar and api-storage Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
✅ Deploy Preview for hyprnote-storybook canceled.
|
✅ Deploy Preview for hyprnote canceled.
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
…cess comment Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
| #[serde(default, deserialize_with = "hypr_api_env::filter_empty")] | ||
| pub supabase_service_role_key: Option<String>, |
There was a problem hiding this comment.
🔴 Duplicate supabase_service_role_key field shadows flattened SupabaseEnv, causing app startup crash
Adding supabase_service_role_key: Option<String> as a named field on Env (line 19) conflicts with the same field inside #[serde(flatten)] pub supabase: hypr_api_env::SupabaseEnv (line 22), which also contains supabase_service_role_key: String (crates/api-env/src/lib.rs:23).
Root Cause
When serde processes #[serde(flatten)], it first matches each key in the input map against explicitly-named fields. The key supabase_service_role_key matches the new top-level Option<String> field and is consumed. The remaining (unmatched) keys are then forwarded to flattened structs. Because the key has already been consumed, SupabaseEnv never sees supabase_service_role_key, and since that field is a required String (not Option), deserialization of SupabaseEnv fails.
This makes envy::from_env().expect("Failed to load environment") at apps/api/src/env.rs:55 panic on startup, crashing the application.
Impact: The API server cannot start at all. This is a complete regression.
Fix: Remove the new supabase_service_role_key field from Env and instead pass Some(env.supabase.supabase_service_role_key.clone()) in main.rs where NangoConfig::new is called, since SupabaseEnv already provides this value as a required field.
Prompt for agents
In apps/api/src/env.rs, remove lines 18-19 (the new supabase_service_role_key field). The field already exists inside the flattened SupabaseEnv struct and having it as a named field on Env shadows it, preventing SupabaseEnv from deserializing.
Then in apps/api/src/main.rs around line 74-78, change:
let nango_config = hypr_api_nango::NangoConfig::new(
&env.nango,
&env.supabase,
env.supabase_service_role_key.clone(),
);
to:
let nango_config = hypr_api_nango::NangoConfig::new(
&env.nango,
&env.supabase,
Some(env.supabase.supabase_service_role_key.clone()),
);
This uses the already-available supabase_service_role_key from SupabaseEnv instead of a duplicate field.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if payload.r#type == "auth" && payload.success && payload.operation != "deletion" { | ||
| state | ||
| .supabase | ||
| .upsert_connection( | ||
| &payload.end_user.end_user_id, | ||
| &payload.provider_config_key, | ||
| &payload.connection_id, | ||
| &payload.provider, | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error = %e, "failed to upsert nango connection"); | ||
| NangoError::Internal(e.to_string()) | ||
| })?; | ||
| } | ||
|
|
||
| // Nango sends deletion webhooks with `success: true` on successful revocation. | ||
| // We gate on `success` to avoid deleting local state if revocation failed on Nango's side. | ||
| if payload.r#type == "auth" && payload.success && payload.operation == "deletion" { | ||
| state | ||
| .supabase | ||
| .delete_connection(&payload.end_user.end_user_id, &payload.provider_config_key) | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error = %e, "failed to delete nango connection"); | ||
| NangoError::Internal(e.to_string()) | ||
| })?; | ||
| } |
There was a problem hiding this comment.
🚩 Webhook handler returns error to Nango on missing service role key
When supabase_service_role_key is None, the service_role_key() method at crates/api-nango/src/supabase.rs:20-26 returns Err(NangoError::Internal(...)). The webhook handler at crates/api-nango/src/routes/webhook.rs:53-79 propagates this as an HTTP error response to Nango. The PR description says "webhooks will log errors but return ok if not set", but the actual behavior is to return an error. Nango may retry the webhook, causing repeated failures. Consider whether the webhook should silently succeed (log + return ok) when the service role key is not configured, rather than returning an error to Nango.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if payload.r#type == "auth" && payload.success && payload.operation != "deletion" { | ||
| state | ||
| .supabase | ||
| .upsert_connection( | ||
| &payload.end_user.end_user_id, | ||
| &payload.provider_config_key, | ||
| &payload.connection_id, | ||
| &payload.provider, | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error = %e, "failed to upsert nango connection"); | ||
| NangoError::Internal(e.to_string()) | ||
| })?; | ||
| } |
There was a problem hiding this comment.
🚩 Webhook assumes end_user.end_user_id is a valid UUID for Supabase FK
The nango_connections table has user_id uuid NOT NULL with a foreign key to auth.users(id) (supabase/migrations/20250214000000_create_nango_connections.sql:14-15). The webhook handler passes payload.end_user.end_user_id (a plain String from crates/nango/src/webhook.rs:42) directly to upsert_connection at crates/api-nango/src/routes/webhook.rs:57. If Nango sends an end_user_id that is not a valid UUID or does not match an existing auth.users.id, the Supabase INSERT will fail with a FK constraint violation, surfacing as an internal error. This depends on how the Nango Connect Session is configured upstream — the caller must ensure end_user.id is set to the Supabase user UUID when creating the session.
Was this helpful? React with 👍 or 👎 to provide feedback.
feat: add NangoConnection<I> extractor for nango-powered integration routes
Summary
Introduces a generic
NangoConnection<I>axum extractor that resolves a user's Nangoconnection_idfrom a Supabasenango_connectionstable, removing the need for clients to passconnection_idin every request body. The extractor uses marker types for type-safe integration identification.Key pieces:
nango_connectionstable) — storesuser_id ↔ connection_idper integration, populated by webhookOwnedNangoProxy+OwnedNangoHttpClientincrates/nango— owned variants to solve lifetime issues in extractors; shared header logic extracted toapply_proxy_headersNangoIntegrationIdtrait +GoogleCalendar/GoogleDrivemarker types incrates/api-nangoNangoConnection<I>extractor — readsAuthContextfor user_id, queries Supabase PostgREST for connection_id, builds ready-to-use HTTP clientSupabaseClientextracted intosupabase.rswith URL-encoded query params andon_conflict=user_id,integration_idfor correct upsert behaviorconnection_idremoved from all request bodies, duplicatedconfig.rs/state.rsdeletedBefore → After (handler example):
Updates since last revision
calendar::router(),nango_connection_stateextension layer).NangoError::Internal, allowing Nango to retry. Previously all errors were silently swallowed with a 200 response.successgate documented: Added comment explaining why we checkpayload.successbefore deleting local state on deletion webhooks — avoids removing a valid local record if revocation failed on Nango's side (per Nango webhook docs, successful deletions setsuccess: true).Review & Testing Checklist for Human
connection_idis removed from all calendar/storage request bodies. No frontend changes are included in this PR — verify that callers are updated or that this is intentional for the new OAuth flow.supabase_service_role_key: With the 500 change, a missing key now causes webhook failures (Nango will retry indefinitely). Confirm this is acceptable for dev environments or add a guard that returns 200 when the key is intentionally unconfigured.NangoConnectionStateis available to calendar/storage routes — the.layer(Extension(...))is added after.nest("/calendar", ...)and.nest("/nango", ...)inapps/api/src/main.rs, which in axum means it applies to all routes above it in the chain. Verify this is correct.nango_connections→ call/calendar/calendarswith just auth token → verify it resolves connection and returns data.Notes
selectpolicy for security. Users can only read their own connections.list_calendarsis now a POST with no request body (just the extractor).OwnedNangoProxyduplicates method signatures fromNangoProxy— this is a known tradeoff to solve lifetime issues in axum extractors. Future proxy behavior changes need to be made in both places.Link to Devin run: https://app.devin.ai/sessions/4feb4eb6926b42c5b1c43ccca430802b
Requested by: @yujonglee